feat: optionally use WASM function names for compiled methods - #168

@andreas-karlsson quick note, please make sure you remove AI tools(co-authored) from the final commit as per policy for this repo.

@andreaTP I'm happy with the implementation so have promoted the PR for review. I haven't tried it with different compiler toolchains though, and I'm not sure I understand the need? If we see this as a feature to just get better readable output when inspecting stacktraces etc. then I don't think we can do better than replacing every disallowed char with an underscore. The possibility to extract the original func id remains as an escape hatch to retrieve the exact function name.

@andreas-karlsson thanks a lot for this PR and for keeping improving the codebase!

I'm not sure I understand the need?

To validate it actually improves readability or produce something that is useful.

I don't think we can do better than replacing every disallowed char with an underscore

This means that, once we get a function name, coming, for example, from Rust we won't be able to re-construct the original name by running rust-demangle on it.

I recall that Rust mangled function names easily become less readable if mangled again.

Thanks @andreaTP, always happy to help! And this is something we'd need for our latest developments.

This means that, once we get a function name, coming, for example, from Rust we won't be able to re-construct the original name by running rust-demangle on it.

Indeed, and that is the sticking point. Is this a feature to get best effort (human) readable stacktraces, or is it intended for tooling? I think both are difficult to achieve at once. If we introduce escaping, mangled names will definitely look even more mangled. I think we have the following options:

• Promote readability, but also keep the function index parsable. The way to recover the original name is by parsing the function id and having access to the WASM (or at least the name section). This is what this PR does.
• Promote name fidelity. We need to escape (not santize) disallowed chars. I think the best bet would be URL encoding, as it's a standard and the percent sign is allowed by the JVM. Mangled names will definitely become less readable, but also easily recoverable. I think in this case the cleanest would be if we can avoid encoding the function index. I.e. url decoding the name gives you the original directly.

Or do you see another option? If we want to keep it open we can include a naming strategy option instead to allow both?

Here's a concrete proposal for a more generic solution. We add withNamingStrategy and introduce the enum CompilerNamingStrategy with the following values:

• INDEXED This is the default and the fallback if no name exists. Functions are named func_<id>.
• SANITIZED This is what's in this PR. Any disallowed chars are replaced with underscore. We still need to append the function id as names might now collide.
• URL_ENCODED Any disallowed char is simply replaced by percent encoding and no index suffix is added.

So far this only applies to methods but I think later it should also be done for locals and args. The above options should still be compatible. Let me know if you'd prefer a PR in this direction.

Thanks for the update @andreas-karlsson !

I think we are going in the right direction, sharing a couple of things that come to mind:

• mangling the second time already mangled strings is going to result in very bad results (experienced it in w9s)
• does the rename impacts backward compatibility? (e.g. is it ok to ship in a minor release?)

What about making the mangling completely configurable user side(e.g. a SPI or similar)? is it a too big of a change? and providing a few default classes already.
This would make it possible, for example, to hook up rustc-demangler when needed and obtain the best possible result.
Wdyt? I'm afraid to increase too much the scope of the PR.

I got excited about opening this up to user-defined naming and different encodings, but thinking it through I now believe there are two separate problems here.

Class files have proper debugging name tables for parameters and locals (MethodParameters, LocalVariableTable). They're informational — unverified, and with no uniqueness requirement — so they can carry names essentially as-is. There's no equivalent for method names, but the answer to that isn't to smuggle a debug section into the method names themselves.

The hard constraint is that the WASM name section may contain duplicates, so any naming scheme has to add extraneous information to disambiguate. The natural choice is the WASM function id. That's also what gives tooling the most to work with: from the id you can look up the original name in the name section and demangle it properly — strictly better fidelity than anything we could encode into a method name.

From a pure tooling perspective the best method name is arguably just the id, as in WASM itself, and func_<id> is already a fine version of that. We could stop there.

So the question I'm posing is narrower: do we want something more recognizable alongside the id? Thread dumps are a key production debugging tool and have no standard symbolization step, so the method name is all you get. For that we could replace "func" with a sanitized name when one is available — the contract being that the name part is a human hint and tools should never parse it. They operate on the id.

Given that, I'd drop the naming-strategy/SPI direction for now; there's nothing for a custom strategy to recover that the id doesn't already give you. And since debug names stay off by default, generated names are unchanged for existing users — so this should be safe in a minor release.

@andreas-karlsson now I see where you are going 🙂 .

For reference, this is probably the closest incarnation of your request, please have a look and let me know.

Thanks @andreaTP, that's helpful context, both the linked PR and the one it was closed in favour of.

While those PRs solve for original names in stack traces, they also make clear there's an orthogonal problem. Neither PR modifies the compiled function names, so would be of no help when taking a thread dump or profiling a JVM in production.

That's the case I'm trying to solve for. Before feeling confident using Endive in a top-tier service, we'd like to ensure standard operational procedures still work. Ideally it should be possible to take a thread dump and immediately get an idea of what each thread is up to, without having to find the original WASM and consult other tooling. The only way to achieve that with standard tooling is through readable method names.

I think the two PRs drive home one point though. It must always be possible to extract the WASM fn id from the method name, and that is part of the public contract.

If you think readable method names have merit, I propose the following changes:

The compiler gets an option withMethodPrefixer, where MethodPrefixer is a functional interface containing String getMethodPrefix(int id, WasmModule module). The default implementation just returns "func", and MethodPrefixer.fromNameSection() is provided for the common case of using the module's name section. For each method the interface is consulted. Disallowed characters are replaced with underscore, and the suffix _<id> is appended to produce the method name. This has the following benefits:

• Method names are unique by construction.
• User controls readability, compiler controls correctness.
• It's simple to extract the WASM fn id by parsing the _<id> suffix.
• It's possible to avoid the sanitization by steering clear of the disallowed chars, for example by URL-encoding.
• It's future proof. It wouldn't interfere with the stack-trace improvement in your linked PRs, but would also complement them by offering a prefixer that sources readable names from DWARF data.
• It's backwards compatible. If no prefixer is configured, there are no observable differences.

I think an interface option is preferable over SPI. For instance you might want to use different strategies for different compilations in the same application. Although, if SPI is desirable, we could check for it when the option is not provided.

I've updated the PR to reflect this proposal. Looking forward to hearing your thoughts.